feat(CLI): improved nested stack diff #29172

comcalvi · 2024-02-19T22:44:13Z

Issue # 27238

Reason for this change

The existing nested stack diff places a fake property, NestedTemplate, in the templates to be diffed. This prevents displaying resource replacement information in the diff, like we do for top level stacks. This PR does not add changeset replacement information from changesets, but it does add replacement information from the spec.

Description of changes

Reworked nested stack diff to treat nested stacks as top level stacks. This improves the visual UX and sets us up for using changesets with nested stacks.

Before

After

Description of how you validated changes

Unit tests + manual tests.

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

… case this other idea doesn't work

… just in case this other idea doesn't work" This reverts commit 10c86cc.

… save it just in case this other idea doesn't work"" This reverts commit e1e4d34.

…far, but save it just in case this other idea doesn't work""" This reverts commit f793503.

…oach so far, but save it just in case this other idea doesn't work"""" This reverts commit 26eeb68.

…ong approach so far, but save it just in case this other idea doesn't work""""" This reverts commit 7725f60.

…y the wrong approach so far, but save it just in case this other idea doesn't work"""""" This reverts commit aa3e4c7.

… probably the wrong approach so far, but save it just in case this other idea doesn't work""""""" This reverts commit 61ff29f.

…o delete the NestedTemplate thing and store the nested templates some other way

This reverts commit 61ff29f.

This reverts commit 441993e.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

colifran

LGTM -- nice job!

mergify · 2024-02-21T16:47:39Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

rix0rrr · 2024-02-23T08:50:08Z

packages/aws-cdk/lib/api/nested-stack-helpers.ts

-export interface TemplateWithNestedStackCount {
-  readonly deployedTemplate: Template;
-  readonly nestedStackCount: number;
+export interface RootTemplateWithNestedStacks {


Can RootTemplateWithNestedStacks and NestedStackTemplates not be the same structure?

I decided not to because the functions that use this interface as a return type only get the generated templates of nested stacks, not the top level root stack. The functions that take these objects as input have a different way of getting the top-level generated template (namely, they take a StackArtifact that contains it). They need something special that the StackArtifact provides, so I decided to keep the generated template out of this data structure, but only for the top-level artifacts.

We could mark generatedTemplate as optional, but that's less clear than using the separate type.

aws-cdk-automation · 2024-03-13T19:30:05Z

➡️ PR build request submitted to test-main-pipeline ⬅️

A maintainer must now check the pipeline and add the pr-linter/cli-integ-tested label once the pipeline succeeds.

mergify · 2024-03-13T19:33:29Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2024-03-13T20:00:34Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 918831c
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2024-03-13T20:20:16Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

reverts #29394, which prevented changeset creation during `cdk diff` if a stack did not exist. The lookup of the stack to check its existence is failing for customers that have CI/CD that won't assume the deploy role when running CDK diff. Long-term fix: delete the stack if it didn't exist before we created the changeset, but wait for its state to reach `DELETE_COMPLETE` to avoid problems with subsequent commands. Preserves changes from #29172 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

### Issue # (if applicable) ### Reason for this change The existing nested stack diff places a fake property, `NestedTemplate`, in the templates to be diffed. This prevents displaying resource replacement information in the diff, like we do for top level stacks. This PR does *not* add changeset replacement information from changesets, but it does add replacement information from the spec. ### Description of changes Reworked nested stack diff to treat nested stacks as top level stacks. This improves the visual UX and sets us up for using changesets with nested stacks. #### Before <img width="957" alt="Screenshot 2024-02-19 at 1 47 59 PM" src="https://github.com/aws/aws-cdk/assets/66279577/a94275c4-e7c3-4d2c-a924-ee61c36bea4d"> #### After <img width="957" alt="Screenshot 2024-02-19 at 1 48 48 PM" src="https://github.com/aws/aws-cdk/assets/66279577/5263aaf9-ef2f-4228-b413-81e780c4b8f8"> ### Description of how you validated changes Unit tests + manual tests. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

comcalvi added 30 commits January 4, 2024 16:01

working create changeset

7543775

working nested stack parsing, I think...

441993e

fix the thing...

c2fe021

reset point, in case I break it all...

d89866d

nested diffs\!

d26bbf0

temp. This is probably the wrong approach so far, but save it just in…

10c86cc

… case this other idea doesn't work

Revert "temp. This is probably the wrong approach so far, but save it…

e1e4d34

… just in case this other idea doesn't work" This reverts commit 10c86cc.

Revert "Revert "temp. This is probably the wrong approach so far, but…

f793503

… save it just in case this other idea doesn't work"" This reverts commit e1e4d34.

Revert "Revert "Revert "temp. This is probably the wrong approach so …

26eeb68

…far, but save it just in case this other idea doesn't work""" This reverts commit f793503.

Revert "Revert "Revert "Revert "temp. This is probably the wrong appr…

7725f60

…oach so far, but save it just in case this other idea doesn't work"""" This reverts commit 26eeb68.

Revert "Revert "Revert "Revert "Revert "temp. This is probably the wr…

aa3e4c7

…ong approach so far, but save it just in case this other idea doesn't work""""" This reverts commit 7725f60.

Revert "Revert "Revert "Revert "Revert "Revert "temp. This is probabl…

61ff29f

…y the wrong approach so far, but save it just in case this other idea doesn't work"""""" This reverts commit aa3e4c7.

Revert "Revert "Revert "Revert "Revert "Revert "Revert "temp. This is…

d70a42a

… probably the wrong approach so far, but save it just in case this other idea doesn't work""""""" This reverts commit 61ff29f.

well, nested stacks have been pulled out. Now it looks like we need t…

5fbd0e8

…o delete the NestedTemplate thing and store the nested templates some other way

this alsmost fixed it

a7c7303

no more too long pipes from objectDiff

e7c8927

working new nested stack diff

1776ad4

revert...

9f33ed0

Revert

e7fa75c

This reverts commit 61ff29f.

revert

9943b1e

revert

4ef5478

revert

32a8357

revert

c849e84

revert

b4e916e

revert

ddb5b07

revert

b03328a

revert

71d9629

Revert "working nested stack parsing, I think..."

1cc79b9

This reverts commit 441993e.

revert

d5db852

refactor

3f4fb0c

aws-cdk-automation added pr/needs-maintainer-review This PR needs a review from a Core Team Member and removed pr/needs-cli-test-run This PR needs CLI tests run against it. labels Feb 20, 2024

dead code

be179ab

comcalvi had a problem deploying to test-pipeline February 20, 2024 23:19 — with GitHub Actions Failure

comment

7d9839d

comcalvi had a problem deploying to test-pipeline February 20, 2024 23:21 — with GitHub Actions Failure

colifran approved these changes Feb 21, 2024

View reviewed changes

mergify bot had a problem deploying to test-pipeline February 21, 2024 16:48 Failure

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Feb 21, 2024

rix0rrr reviewed Feb 23, 2024

View reviewed changes

github-actions bot mentioned this pull request Mar 1, 2024

Monthly PR metrics report - Feb 2023 #29328

Closed

comcalvi added 2 commits March 13, 2024 09:27

conflicts

6be7f06

add import test

2c9dd9e

comcalvi force-pushed the comcalvi/improved-nested-stack-diff branch from 5e00582 to 2c9dd9e Compare March 13, 2024 19:01

comcalvi temporarily deployed to test-pipeline March 13, 2024 19:02 — with GitHub Actions Inactive

Merge branch 'main' into comcalvi/improved-nested-stack-diff

918831c

mergify bot had a problem deploying to test-pipeline March 13, 2024 19:34 Failure

mergify bot merged commit 135b520 into main Mar 13, 2024
11 of 12 checks passed

mergify bot deleted the comcalvi/improved-nested-stack-diff branch March 13, 2024 20:20

comcalvi mentioned this pull request Mar 14, 2024

revert: prevent changeset diff for non-deployed stacks #29485

Merged

This was referenced May 23, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#98

Open

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#99

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(CLI): improved nested stack diff #29172

feat(CLI): improved nested stack diff #29172

comcalvi commented Feb 19, 2024 •

edited by SankyRed

Loading

colifran left a comment

mergify bot commented Feb 21, 2024

rix0rrr Feb 23, 2024

comcalvi Mar 13, 2024

aws-cdk-automation commented Mar 13, 2024

mergify bot commented Mar 13, 2024

aws-cdk-automation commented Mar 13, 2024

mergify bot commented Mar 13, 2024

feat(CLI): improved nested stack diff #29172

feat(CLI): improved nested stack diff #29172

Conversation

comcalvi commented Feb 19, 2024 • edited by SankyRed Loading

Issue # 27238

Reason for this change

Description of changes

Before

After

Description of how you validated changes

Checklist

colifran left a comment

Choose a reason for hiding this comment

mergify bot commented Feb 21, 2024

rix0rrr Feb 23, 2024

Choose a reason for hiding this comment

comcalvi Mar 13, 2024

Choose a reason for hiding this comment

aws-cdk-automation commented Mar 13, 2024

mergify bot commented Mar 13, 2024

aws-cdk-automation commented Mar 13, 2024

AWS CodeBuild CI Report

mergify bot commented Mar 13, 2024

comcalvi commented Feb 19, 2024 •

edited by SankyRed

Loading